Skip to content

[PM-38792] feat: Enforce Send deletion date via Send Controls policy - #2910

Open
matt-livefront wants to merge 2 commits into
matt/PM-40460-send-controls-restrict-send-typefrom
matt/PM-38792-send-controls-restrict-deletion-days
Open

[PM-38792] feat: Enforce Send deletion date via Send Controls policy#2910
matt-livefront wants to merge 2 commits into
matt/PM-40460-send-controls-restrict-send-typefrom
matt/PM-38792-send-controls-restrict-deletion-days

Conversation

@matt-livefront

@matt-livefront matt-livefront commented Jul 24, 2026 •
edited by atlassian Bot
Loading

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

PM-38792

📔 Objective

Adds enforcement of the new Send Controls policy deletionHours option. When an organization restricts the Send deletion date, a non-owner/non-admin user sees the enforced deletion date pre-selected and locked on the Send add/edit screen, with helper text explaining the restriction. Also adds the previously-missing 14 days deletion option to the menu.

📸 Screenshots

Screenshot 2026-07-24 at 12 18 27 PM

@matt-livefront
matt-livefront requested a review from a team as a code owner July 24, 2026 19:04
@matt-livefront matt-livefront added ai-review Request a Claude code review t:feature labels Jul 24, 2026
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context app:authenticator Bitwarden Authenticator app context labels Jul 24, 2026
@github-actions

github-actions Bot commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the Send Controls deletionHours enforcement across SendPolicyOptions (minimum-hours resolution across applying policies), the new SendDeletionDateType.fourteenDays case and from(hours:) mapping, and the add/edit Send screen (state, processor, view). The implementation mirrors the existing enforcedAccessType pattern, and the feature flag gate lives in PolicyService.getSendPolicyOptions() so enforcedDeletionDateHours is only populated when sendControls is enabled. Test coverage is thorough: preset mapping, custom fallback, multi-policy resolution, processor load behavior, and the disabled/helper-text view states.

Code Review Details

No blocking findings.

  • ♻️ : Pre-existing (not introduced here, noted for awareness) — removePassword replaces the whole state with AddEditSendItemState(sendView:), which resets sendPolicyOptions and isSendControlsPolicyEnabled to defaults. After removing a password in edit mode the newly added deletion-date enforcement (and the access-type enforcement from the base branch) silently drops until the screen reloads. Carrying the policy fields onto newState alongside isOptionsExpanded would preserve them.
    • BitwardenShared/UI/Tools/Send/SendItem/AddEditSendItem/AddEditSendItemProcessor.swift:291

Notes considered and cleared during review:

  • In edit mode loadData overwrites the loaded Send's deletion date with the policy-enforced date, which can shift an existing Send's expiry when only the name is edited. This is consistent with how enforcedAccessType is applied and keeps the saved value within the policy limit.
  • A non-preset enforced value yields a .custom date that is absent from availableDeletionDateTypes in .add mode; BitwardenMenuField renders selection.localizedName directly and the field is disabled, so it displays correctly.
  • AnyCodable.intValue handles int/double/string server encodings for deletionHours.

@codecov

codecov Bot commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.61%. Comparing base (71faad8) to head (61dc7db).

Additional details and impacted files 
@@                                Coverage Diff                                 @@
##           matt/PM-40460-send-controls-restrict-send-type    #2910      +/-   ##
==================================================================================
+ Coverage                                           79.58%   79.61%   +0.02%     
==================================================================================
  Files                                                1170     1170              
  Lines                                               75321    75418      +97     
==================================================================================
+ Hits                                                59947    60041      +94     
- Misses                                              15374    15377       +3

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@matt-livefront
matt-livefront force-pushed the matt/PM-38792-send-controls-restrict-deletion-days branch from 4a790d9 to e6e82db Compare August 21, 2026 19:46
fedemkr
fedemkr reviewed Aug 21, 2026
View reviewed changes

@fedemkr fedemkr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a ⛏️

Comment thread BitwardenResources/Localizations/en.lproj/Localizable.strings Outdated
"NewSendCreated" = "Send created";
"Custom" = "Custom";
"SendDisabledWarning" = "Due to an enterprise policy, you are only able to delete an existing Send.";
"SendDeletionDateEnforcedByOrganization" = "This date is enforced by your organization";

@fedemkr fedemkr Aug 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏️ Can't we use the key made from the value? ThisDateIsEnforcedByYourOrganization

@matt-livefront matt-livefront Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good catch!

@matt-livefront
matt-livefront force-pushed the matt/PM-38792-send-controls-restrict-deletion-days branch from e6e82db to 61dc7db Compare August 24, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fedemkr fedemkr fedemkr left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

ai-review Request a Claude code review app:authenticator Bitwarden Authenticator app context app:password-manager Bitwarden Password Manager app context t:feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@matt-livefront @fedemkr